Pattern selection #792

lslezak · 2023-10-04T15:12:31Z

Problem

User cannot change the default software selection in Agama, it always installs a minimal system
That is annoying for beginner users esp. in Leap or Tumbleweed installations where you want to have at least a minimal desktop system after installation

Solution

Allow selecting the available patterns, that's simpler and easier than a full package manager (for both users and developers)

Testing

Tested manually

TODO

Display solver errors if there are any
Unit tests
Comments in code
TRANSLATORS comments
Fix pattern group ordering to be the same as in YaST (needs discussion with @shundhammer, maybe later in a separate PR)

Screenshots

github-actions · 2023-10-04T15:17:19Z

coverage: 74.148% (-0.9%) from 75.084% when pulling 9b0e480 on pattern_selection into 898f681 on master.

joseivanlopez · 2023-10-04T15:35:22Z

I think this is a movement in the good direction. Now it looks similar to the rest of pages. Personally, I would remove the first 4 lines, resulting something like:

[ filter ]

Graphical environments
[x] Gnome
[ ] KDE

Dev tools
[ ] Node JS

What do you think?

shundhammer · 2023-10-04T16:35:27Z

@lslezak: FYI: patterns have a (numeric IIRC) ZyppPattern::order() that is used for sorting them. See also

shundhammer · 2023-10-04T16:42:45Z

Personally, I would remove the first 4 lines

I wouldn't remove the total size of the software to install. That affects remaining disk space for working with the system as well as download times.

As for the number of patterns to install, I don't know. It could be in one line to be a bit more concise:

Selected 4 patterns with 695.8 MiB total

Maybe the "Filter" heading could be a bit more subdued to avoid the impression that it's a category on the same level as the others; it's really just a tool to work on the others. Maybe a smaller font or a greyish text color would make that clearer (needs experimentation).

lslezak · 2023-10-04T17:53:45Z

@lslezak: FYI: patterns have a (numeric IIRC) ZyppPattern::order() that is used for sorting them.

Yes, I wanted to implement the same also here. Sorting the patterns inside a group is trivial, I have problems with sorting the groups themselves.

In TW with Online repos I'm getting this order in YaST:

While in Agama I'm getting:

Graphical Environments
Documentation
Base Technologies
Desktop Functions
Server Functions
Development
KDE Desktop
Budgie Desktop
Containers
Primary Functions
Proprietary Software
MicroOS
Kubernetes
Aeon

It seems that libyui should sort the groups according to the first pattern in the group (the pattern with the lowest order value). But for some reason the result is slightly different than expected and actually the Agama result matches that expected behavior.

In libyui for example the Documentation group (orders 1005 and 5200) is sorted between Base Technologies (orders 1030...5129) and Containers (orders 3000 and 9030) groups. Which looks strange, the final first pattern order is 1030, 1005, 3000.

I think it should be placed between the Graphical Environments (orders 1000...1802) and Base Technologies (orders 1030...5129) as in the Agama ordering.

@shundhammer any idea why the libyui sorting is like that?

(Note: the ZyppPattern::order() returns std::string so the values are compared lexicographically, not numerically. But that should not matter, all existing patterns use 4 digit values so that should work fine.)

lslezak · 2023-10-04T18:11:50Z

I would remove the first 4 lines

The number of selected patterns is not that important, I'm fine with removing that. In the worst case you can still count them manually if you need that. But the installation size is important, it determines how much time the installation will take, how much data will be downloaded (if you have slow network or pay per megabyte) and the available free space after installation.

You can see that number in the overview dialog after going back, but I think it is better to see immediately how your each change influence the installation size.

dgdavid

First of all, thanks a lot for taking care of this @lslezak!

I've done a review focusing mostly in the UI end result, omitting some parts I foresee will change in a near future like the styles for building the selector, the way we tell the user there is something auto-selected and few other minor things. As you told me, it's still not finished and, moreover, it's good enough for a first release that can be improved in next iterations.

Trying to keep it short, in no particular order

I wouldn't use title for the utility section. It is an introductory area of the Software page in which we can put useful information and some utils, like the search/filter box.
I'd avoid mixin the use of sections icons in a same Page. I.e., icons for all or for none. At least by now. See my comment in that regard.
Do not use a header for the filter. Just a plain label or we can even try to release it only with the placeholder.
Keep the Software overview section clickable always. The page should be aware if the client is busy and rendered itself in the loading mode. Users can land directly in https://agama/#/software.
Use the core/Section component as much as possible. Improve it if needed or create an issue if you believe these improvements do not necessary block this PR and are out of its scope.

I've deliberately skipped the logic part for building the whole page and making it work. No particular reason, I simply need more time for getting familiar with it and been able to come up with a good review/contribution. In any case, please add tests. They will help to prevent regression bugs and to have a big picture about pattern selection.

web/package.json

web/src/assets/styles/app.scss

web/src/components/overview/SoftwareSection.jsx

web/src/components/software/PatternGroup.jsx

web/src/components/software/PatternItem.jsx

web/src/components/software/PatternSelector.jsx

web/src/components/software/SoftwareSelectionPage.jsx

dgdavid · 2023-10-04T23:33:03Z

Maybe the "Filter" heading could be a bit more subdued to avoid the impression that it's a category on the same level as the others; it's really just a tool to work on the others. Maybe a smaller font or a greyish text color would make that clearer (needs experimentation).

As commented, I think simply removing the header and using either, an HTML form label or only the text input placeholder, would be enough. Do not introduce more font sizes or color just for this.

shundhammer · 2023-10-09T15:42:57Z

JFTR: It turned out that libyui-qt-pkg had done this wrong; it always picked the last pattern for the order of its parent category, not the first one as it should.

The fix for that is on the way at libyui/libyui#107 (which also includes debugging aids such as making the order of each pattern and category visible in its own column when a environment variable is set).

improved group sorting, use String for pattern order

service/package/rubygem-agama.changes

web/package/cockpit-agama.changes

joseivanlopez · 2023-10-11T08:49:36Z

web/src/components/software/PatternGroup.jsx

+ * @param {JSX.Element} children the wrapped content with the patterns belonging to this group
+ * @returns {JSX.Element}
+ */
+export default function PatternGroup({ name, children }) {


NP: does it deserve its own component?

Originally it was much more complex because it was a collapsible/expandable section. But I'd probably still keep it separate as it is a self contained logical part.

joseivanlopez · 2023-10-11T09:02:17Z

web/src/components/software/PatternSelector.jsx

+ * @property {string} description long description of the pattern
+ * @property {string} order display order (string!)
+ * @property {string} icon icon name (not path or file name!)
+ * @property {number|undefined} selected who selected the pattern, undefined


NP: I usually indicate optional property instead of undefined, e.g. @property {number} [selected] - who selected .... We should agree a common format.

I copied that from other code so it seems some unification is needed...

web/src/components/software/PatternSelector.jsx

web/src/components/software/SoftwarePage.jsx

joseivanlopez · 2023-10-11T09:53:40Z

Just for the records: as discussed at some point, the selection icons should be revisited (something for a next iteration). Mainly for the auto-selected icon with subtle dots. Using labels was proposed as a possible solution.

lslezak · 2023-10-11T14:12:02Z

Just for the records: as discussed at some point, the selection icons should be revisited (something for a next iteration). Mainly for the auto-selected icon with subtle dots. Using labels was proposed as a possible solution.

Yes, I just reused the same icons from YaST for the first iteration, I didn't want to reinvent the wheel. Later we can change that with something better.

dgdavid reviewed Oct 4, 2023

View reviewed changes

lslezak force-pushed the pattern_selection branch 10 times, most recently from e8ff8e9 to 687b0b5 Compare October 9, 2023 08:29

lslezak added 11 commits October 10, 2023 09:32

initial pattern selector

8144d28

Group patterns

f9a1aae

Update pattern selector

23ce6e0

Update state

300a77b

Display patten icons, refresh the counters

119f05f

Use status icons from libyui-qt-pkg

e0641af

Show pattern selector after repositories are initialized

c169894

Display used size

0753eae

Added search filter, pattern groups not collapsible

f918afa

improved group sorting, use String for pattern order

initialize selected patterns

c560320

cleanup

6a0aa10

lslezak added 12 commits October 10, 2023 09:33

Update spellchecker

6c8f89a

Code review improvements

a9988e8

jsdoc comments

e967000

Try deselecting the autoselected patterns, unit tests

53c3a0e

More tests

4cf1e09

Better example

a94eb69

Unit test

0a1c0ab

Update test

28d5e9d

Update test

0716cf6

Improved sorting, added tests

4b15386

Fixed spell check, added unit test

0df2919

Resolve conflict

b0cb432

lslezak force-pushed the pattern_selection branch from fb5a04c to b0cb432 Compare October 10, 2023 08:35

Changes

f1f5ec8

lslezak marked this pull request as ready for review October 10, 2023 08:53

Remove unused import

0f67b43

joseivanlopez reviewed Oct 11, 2023

View reviewed changes

lslezak added 2 commits October 11, 2023 16:00

Use null instead of empty fragments

2ac7a3f

Updated changes

6d8947c

joseivanlopez approved these changes Oct 11, 2023

View reviewed changes

lslezak merged commit 99ab3e4 into master Oct 11, 2023
11 checks passed

lslezak deleted the pattern_selection branch October 11, 2023 14:29

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Pattern selection #792

Pattern selection #792

lslezak commented Oct 4, 2023 •

edited

Loading

github-actions bot commented Oct 4, 2023

joseivanlopez commented Oct 4, 2023

shundhammer commented Oct 4, 2023 •

edited

Loading

shundhammer commented Oct 4, 2023

lslezak commented Oct 4, 2023 •

edited

Loading

lslezak commented Oct 4, 2023

dgdavid left a comment •

edited

Loading

dgdavid commented Oct 4, 2023

shundhammer commented Oct 9, 2023 •

edited

Loading

joseivanlopez Oct 11, 2023

lslezak Oct 11, 2023

joseivanlopez Oct 11, 2023

lslezak Oct 11, 2023

joseivanlopez commented Oct 11, 2023

lslezak commented Oct 11, 2023

Pattern selection #792

Pattern selection #792

Conversation

lslezak commented Oct 4, 2023 • edited Loading

Problem

Solution

Testing

TODO

Screenshots

github-actions bot commented Oct 4, 2023

joseivanlopez commented Oct 4, 2023

shundhammer commented Oct 4, 2023 • edited Loading

shundhammer commented Oct 4, 2023

lslezak commented Oct 4, 2023 • edited Loading

lslezak commented Oct 4, 2023

dgdavid left a comment • edited Loading

Choose a reason for hiding this comment

dgdavid commented Oct 4, 2023

shundhammer commented Oct 9, 2023 • edited Loading

joseivanlopez Oct 11, 2023

Choose a reason for hiding this comment

lslezak Oct 11, 2023

Choose a reason for hiding this comment

joseivanlopez Oct 11, 2023

Choose a reason for hiding this comment

lslezak Oct 11, 2023

Choose a reason for hiding this comment

joseivanlopez commented Oct 11, 2023

lslezak commented Oct 11, 2023

lslezak commented Oct 4, 2023 •

edited

Loading

shundhammer commented Oct 4, 2023 •

edited

Loading

lslezak commented Oct 4, 2023 •

edited

Loading

dgdavid left a comment •

edited

Loading

shundhammer commented Oct 9, 2023 •

edited

Loading